Check for a APT Release file in the github workflow: #58
Conversation
-The goal is to do more than check if the Mirror is simply 'online', but actually useable - WIP to resolve armbian#56
WalkthroughThe workflow script for generating the redirector configuration was updated to improve how mirror usability is determined. The new logic checks for the presence of at least one "Release" file in the downloaded "compare/dists" directory for each mirror. If no Release file is found, a "no_release" status is recorded for that server, and this status is written to a file and set as an environment variable. Additionally, the workflow's summary output now includes a section listing all servers with the "no_release" status, making missing Release files explicitly visible in the summary. Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found. All modifications are directly related to the objectives stated in the linked issue, focusing on mirror usability checks and reporting within the workflow logic. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
.github/workflows/generate-redirector-config.yml (4)
255-261: Same un-initialisedexit_status/findissue as above – please replicate the fix suggested for lines 190-196.
461-463: Samegrepfailure mode as commented at 427-429 – please apply the same guard.
494-496: Samegrepfailure mode as commented at 427-429 – please apply the same guard.
528-530: Samegrepfailure mode as commented at 427-429 – please apply the same guard.
| echo "# No Release file" >> $GITHUB_STEP_SUMMARY | ||
| grep no_release status/* | cut -d":" -f1 | xargs awk 'FNR==2{print}' >> $GITHUB_STEP_SUMMARY | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
grep … | awk pipeline will fail when there are no no_release files
If no mirror is missing a Release file grep exits 1 and, with pipefail, the whole step fails. Add a fallback:
-grep no_release status/* | cut -d":" -f1 | xargs awk 'FNR==2{print}' >> $GITHUB_STEP_SUMMARY
+grep -q no_release status/* 2>/dev/null && \
+ grep no_release status/* | cut -d":" -f1 | xargs awk 'FNR==2{print}' >> $GITHUB_STEP_SUMMARY || trueRepeat for the three other summary blocks (lines 461-463, 494-496, 528-530).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "# No Release file" >> $GITHUB_STEP_SUMMARY | |
| grep no_release status/* | cut -d":" -f1 | xargs awk 'FNR==2{print}' >> $GITHUB_STEP_SUMMARY | |
| echo "# No Release file" >> $GITHUB_STEP_SUMMARY | |
| grep -q no_release status/* 2>/dev/null && \ | |
| grep no_release status/* | cut -d":" -f1 | xargs awk 'FNR==2{print}' >> $GITHUB_STEP_SUMMARY || true |
🤖 Prompt for AI Agents
In .github/workflows/generate-redirector-config.yml around lines 427 to 429, the
pipeline using grep and awk fails when no files match "no_release" because grep
exits with status 1, causing the step to fail under pipefail. To fix this,
modify the command to provide a fallback that prevents failure when no matches
are found, such as appending "|| true" after the grep pipeline. Apply the same
fix to the similar summary blocks at lines 461-463, 494-496, and 528-530.
| # Check for at least one Release file | ||
| RELEASE_FOUND=$(find compare/dists -type f -name Release | wc -l) | ||
| if [[ "$RELEASE_FOUND" -eq 0 ]]; then | ||
| echo "no_release" >> status/${SERVER_ID} | ||
| echo "${SERVER_URL}" >> status/${SERVER_ID} | ||
| echo "STATUS=no_release" >> $GITHUB_ENV | ||
| elif [[ -z "${OUT}" ]]; then |
There was a problem hiding this comment.
exit_status is never initialised & find can break ‑ leads to false classifications
exit_statusis referenced later in the samecaseclause, but it is never set in this job (thelftpcall is followed by|| true, discarding its exit code). Every server that reaches this branch will therefore evaluate[[ "${exit_status}" -eq 0 ]]as[[ -eq 0 ]](empty → 0) and be markednot_in_synceven when the mirror is fine.find compare/dists …errors out (status = 1) whencompare/distsdoes not exist (e.g. 404/timeout). Withset -e -o pipefail(GitHub Actions default) that will abort the step.
- RELEASE_FOUND=$(find compare/dists -type f -name Release | wc -l)
+ RELEASE_FOUND=$(find compare/dists -type f \( -name Release -o -name InRelease \) 2>/dev/null | wc -l)And preserve the lftp exit code:
- timeout 5m lftp -e "mirror --parallel=16; exit" https://${SERVER_URL}/dists/ || true
+ timeout 5m lftp -e "mirror --parallel=16; exit" https://${SERVER_URL}/dists/ || exit_status=$? || trueFinally guard the comparison:
- elif [[ "${exit_status}" -eq 0 ]]; then
+ elif [[ -n "${exit_status}" && "${exit_status}" -eq 0 ]]; thenApply the same fixes to every identical block below.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .github/workflows/generate-redirector-config.yml around lines 190 to 196, the
variable exit_status is used without initialization because the lftp command's
exit code is discarded with || true, causing incorrect status classification.
Also, the find command can fail if the compare/dists directory does not exist,
which aborts the step due to set -e -o pipefail. To fix this, capture and
preserve the lftp exit code immediately after the command, avoid using || true,
and modify the find command to handle missing directories gracefully (e.g.,
check if the directory exists before running find or suppress errors). Apply
these changes consistently to all identical blocks in the file.
|
I tested it and it seems resoults are not good. While doing this, a fallback is needed - main repo (rsync.armbian.com) should always be there as empty redirector config will also fail. Now (before this PR) its empty also if no mirror is synched. |
@igorpecovnik can you please review and submit any feedback here.
Thank you